Skip to content

test: currentBearer + couch manager coverage, offline CI tier, pack smoke#245

Merged
vreshch merged 1 commit into
masterfrom
test/coverage-and-ci-tiers
Jul 7, 2026
Merged

test: currentBearer + couch manager coverage, offline CI tier, pack smoke#245
vreshch merged 1 commit into
masterfrom
test/coverage-and-ci-tiers

Conversation

@vreshch

@vreshch vreshch commented Jul 7, 2026

Copy link
Copy Markdown
Member

What

Test/CI hardening only - no src/ runtime files touched (tests + workflows + vitest config).

Coverage

  • Unit-cover currentBearer (src/lib/api.ts): unexpired token returned as-is (zero network), expired token refreshes exactly once and persists the new bearer, refresh failure returns null (no throw), expired-with-no-refresh-token returns null without a network call, and signed-out returns null.
  • Cover the least-tested real logic in src/sync/couch/manager.ts: token-acquisition failure recorded (never thrown), discovery-paused branch, re-entrant cycle no-op, cached wire + target-state reuse, reschedule timer arming, stale-vault state pruning, manual-only (interval 0) target not armed, and the sync-on-save defer branch when signed in but the channel is not couch-ready.

CI tiers

  • New @offline tag on the self-contained e2e files (memory-offline, vault-offline, account-vault, m1-requirements, integrity, mcp-stdio, mcp-daemon, daemon, sync, plus the two offline status smokes). e2e.yml split into e2e-offline (--grep @offline, no live-stack env/secrets - a fast required signal even when dev.agentage.io is down) and e2e-live (the existing full-suite-vs-dev job).
  • ci.yml: added an npm pack smoke job - packs the publish tarball, installs it into a temp prefix, and asserts agentage --version equals package.json and agentage --help exits 0 (catches a broken bin/shebang before publish).
  • ci.yml matrix widened to Node [22.x, 24.x].
  • vitest.config.ts: per-directory coverage floors for src/lib/** (87/78/83/90) and src/sync/** (85/75/75/87), set a few points below the achieved numbers so a weak file cannot hide behind the global average. No floor added for src/commands/** (a sibling PR is churning those).

Why

  • currentBearer is the background-sync bearer for the couch/manager path and was only ~85% covered; the couch manager was the least-covered real module (67.9% stmts / 51.4% funcs) with only Docker/live e2e exercising it.
  • The offline tier gives a fast, network-independent gate; the pack smoke catches publish-breaking packaging bugs; per-dir floors stop coverage regressions in the two engine-adjacent dirs.

Coverage before -> after (statements)

  • src/lib/api.ts: 84.78 -> 97.82
  • src/sync/couch/manager.ts: 67.95 -> 83.42 (funcs 51.42 -> 60.0)
  • Per-dir aggregate now: src/lib/** 90.95 stmts / 83.57 br / 87.59 fn / 93.55 ln; src/sync/** 88.89 / 80.13 / 79.49 / 90.85

Verify

  • npm run verify green (350 unit tests, type-check + e2e type-check + lint + format + build).
  • npm run test:e2e -- --grep @offline green locally: 35 tests / 10 files, and confirmed the grep excludes couch-cloud, couch-sync, setup-oauth, mcp-contract.
  • Pack smoke run locally: installed CLI reports 0.0.3 == package.json, --help exit 0.

Notes

  • status.test.ts has 3 smokes; only the two network-independent ones (--version, signed-out no-stack-trace) got @offline. The third asserts endpoint.reachable === true, so it stays live-only.
  • Did not touch src/** runtime, src/commands/**, daemon source, package.json deps, or publish.yml (siblings own them). Pack smoke is inlined in the workflow, no new package script.

@github-actions

github-actions Bot commented Jul 7, 2026

Copy link
Copy Markdown
Contributor

🎉 PR Validation ✅ PASSED

Commit: 4d21faf3f59b02a9d1bb1d403a24b67cdf8125b0
Branch: test/coverage-and-ci-tiers

Checks:

  • ✅ Release guard (no version/changelog changes)
  • ✅ Dependencies installed
  • ✅ Type check passed
  • ✅ Linting passed
  • ✅ Format check passed
  • ✅ Tests + coverage passed
  • ✅ Build successful

Ready to merge!


🔗 View workflow run
⏰ Generated at: 2026-07-07T22:08:34.202Z

@vreshch
vreshch marked this pull request as ready for review July 7, 2026 22:10
@vreshch
vreshch merged commit 175accf into master Jul 7, 2026
3 checks passed
@vreshch
vreshch deleted the test/coverage-and-ci-tiers branch July 7, 2026 22:10
vreshch added a commit that referenced this pull request Jul 7, 2026
## What

README overhaul plus a hand-authored architecture diagram, aligned to
master after #238-#245. Ready as the npm landing page.

Touches only `README.md` and `docs/**` - no `src/`, `package.json`, or
workflows.

## Changes

- **`docs/architecture.svg`** - hand-written SVG (no render tooling).
Neutral background with dark text on light boxes, so it reads in both
light and dark GitHub themes and on npm. Shows AI tools -> MCP (stdio
`agentage mcp` + daemon HTTP `/mcp` on `127.0.0.1:4243`) -> CLI + local
daemon (single writer) -> local vaults (git-per-vault,
`@agentage/memory-core`) -> sync out to git remotes and account sync
(labeled protocol-neutrally, no internal tech named).
- **`README.md`** restructured for a first-time reader: What is this ->
Install -> Quickstart (local-only first, then optional `agentage setup`)
-> Architecture (diagram) -> Command reference -> Sync -> MCP
integration -> Daemon -> Env vars -> Development.
- **`docs/architecture.md`** - short walk-through of the diagram.

## Reality-check vs master

Built the CLI (`npm run build`) and verified every command and flag
against the live `--help` output. Corrected drift from the merged PRs:

- `vault add` now defaults to an **account** vault; `--local [path]`,
`--git <remote>`, `--path <dir>` are the alternatives.
- `vault sync [name]` now covers git and the account channel; progress
prints per vault.
- Documented the `update` command and `--no-daemon` global flag.
- Daemon section notes the loopback-only, token-guarded,
cross-origin-rejecting API and port-in-use reporting (#243, #244).
- Friendly memory errors + non-zero exits (#240); 64 KB read clamp and
secret refusal.
- Describes no specific `src/` file paths (a sibling PR is moving
`src/sync/` paths).

## Verification

- `npm run verify` green: 405 unit tests pass, type-check + lint +
format:check + build clean.
- Ran the offline quickstart end-to-end against the built `dist/cli.js`
(vault add --local, write via stdin, list, search, read `@vault/path`) -
all pass.
- Rendered `docs/architecture.svg` to PNG and eyeballed legibility.

Note: `format:check` only globs `{src,e2e}/**/*.ts`, so it does not lint
markdown; README changes are prose-only.
vreshch added a commit that referenced this pull request Jul 7, 2026
)

## Root cause (issue #249)

`src/lib/file-lock.ts` decided lock takeover from **age alone**. A
holder that acquires
the lock, then gets descheduled by the OS for longer than `LOCK_TTL_MS`
(10s) - realistic
on a 2-core GitHub runner running 20 child processes **plus** V8
coverage (added to both
lanes in #245; `NODE_V8_COVERAGE` is inherited by the `execFile`
children, so every child
pays the tax) - still has its `<pid> <timestamp>` lock look *stale*.

`acquireFileLock` (old `file-lock.ts:79-81`) then treated that live
holder as crashed:

```
const held = heldAt(lockPath(target));
if (held !== null && now - held < LOCK_TTL_MS) return false; // fresh -> refuse
if (!takeOverStale(target, now)) return false;               // else STEAL it
```

`takeOverStale` deletes the "stale" lock and a second process enters the
critical section
while the original holder is still mid read-modify-write. Both
`readFileSync` the array,
both `push`, the later `writeFileSync` clobbers the earlier - exactly
one append is lost.
The holder was never dead, so it still prints `ok`: hence **all 20
children succeed yet the
array is 19/20** (`expected [0..18] to deeply equal Array(20)`). This is
a **product bug**,
not a test bug.

## Mechanism, reproduced

Standalone harness: the 20-process append race with process 0's critical
section paused
11s (simulating the CPU-starvation the runner produces), against the two
`file-lock.ts`:

```
ORIG  pause=11000  iters=8   lostWrites=8   throwPath=0   # incl. an exact 19/20 iter
FIX   pause=11000  iters=10  lostWrites=0   throwPath=0
```

The original loses an append on **every** iteration (all children still
report `ok`,
matching the CI signature); the fix loses none.

## The fix

Takeover is now gated on **same-host pid liveness**, not age alone
(`file-lock.ts` `isCrashed`):

- A holder is stolen only once it is past the TTL **and** provably gone
- its pid returns
`ESRCH` from `process.kill(pid, 0)`, or it is our own leftover pid (we
cannot be inside
our own synchronous section while trying to acquire), or it is past a
60s hard backstop
  (guards against pid reuse after a genuine crash).
- A live-but-CPU-starved holder is **waited out** by the caller's
bounded backoff, never
stolen. If it is genuinely wedged past `MAX_WAIT_MS`, the caller throws
a loud
`could not acquire lock` - a safe, visible failure, never silent
corruption.

Why correct: the only way two processes can be in the critical section
is if a takeover
fires while the first is alive. Liveness makes that impossible for any
alive holder,
regardless of how long the scheduler pauses it. Crash recovery is
preserved (dead pid ->
takeover) and the ABA takeover guard is unchanged.

The lock file format stays `<pid> <timestamp>`; parsing is tolerant of
legacy single-token
files (parsed as pid 0 -> ages out on the TTL, so old on-disk locks
still recover).

Same hardening applied to `src/lib/update-lock.ts` (same lock family).
It is far less
exposed (10-min TTL, single-writer), but the fix is symmetric and its
parser is now
backward-compatible with existing bare-timestamp `update.lock` files.

## Evidence / verify

- `npm run verify` green (406 tests, 48 files).
- `file-lock.test.ts` looped **20x = 20/20**; **5x pinned to 2 cores**
(`taskset -c 0,1`) = 5/5.
- All existing tests kept, including the fail-fast-on-EACCES cases
(#240) and the 20-process
zero-lost-writes proof. The crashed-holder test now seeds a reaped
(deterministically dead)
pid instead of a hardcoded `999`, so the liveness check is exercised
reliably. Added a test
proving a stale-looking lock whose holder is **alive** is refused, not
stolen.

Closes #249
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant